-
Notifications
You must be signed in to change notification settings - Fork 604
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(std/node): Add base64url encoding support, indexOf, lastIndexOf and includes to Buffer #1636
Conversation
@@ -68,6 +68,7 @@ | |||
"test-buffer-failed-alloc-typed-arrays.js", | |||
"test-buffer-fakes.js", | |||
"test-buffer-from.js", | |||
"test-buffer-indexof.js", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice 👍
@@ -37,7 +37,7 @@ import { | |||
validateAbortSignal, | |||
validateBoolean, | |||
validateFunction, | |||
} from "./_validators.ts"; | |||
} from "./internal/validators.js"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All this renames might seem unnecessary, but this actually solves two problems:
- _validators is actually a shim of internal/validators however it was left in that location in the early days of std/node, now it will be easier to find the required functions when porting modules
- This solves a circular dependency created between _validators and internal/validators by moving the content of _validators
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, it was bothering me too; good change.
node/_tools/suites/common/dns.js
Outdated
@@ -5,10 +5,10 @@ | |||
// Taken from Node 16.13.0 | |||
// This file is automatically generated by "node/_tools/setup.ts". Do not modify this file manually | |||
|
|||
'use strict'; | |||
"use strict"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no idea why this formatting wasn't being applied, all I did was run setup.ts and all this changes were applied
Ready for review. Since this apparently also formatted some files unintentionally I gotta point out the relevant files:
|
Any idea why this happened? Changed files have |
I have no idea what's going on :/ It doesn't seem to be |
Ok, I found out what was going on. Apparently I ran deno fmt and it formatted the cached decompressed versions of the file as well. Should be fine now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you @Soremwar
No description provided.